Skip to content

Implement ISOSDacInterface::GetObjectClassName in cDAC SOSDacImpl#124816

Open
Copilot wants to merge 6 commits intomainfrom
copilot/implement-get-object-class-name
Open

Implement ISOSDacInterface::GetObjectClassName in cDAC SOSDacImpl#124816
Copilot wants to merge 6 commits intomainfrom
copilot/implement-get-object-class-name

Conversation

Copy link
Contributor

Copilot AI commented Feb 24, 2026

  • Implement GetObjectClassName in SOSDacImpl.cs
  • Add unit tests for GetObjectClassName in ObjectTests.cs
  • Add dump tests in RuntimeTypeSystemDumpTests.cs
  • Address review feedback: rename variables, remove free object special case
  • Revert @object back to objectContract variable name
  • Investigate CI build break
    • cDAC_windows_x64_release SOS.UnitTests failure is pre-existing on main (all 5 recent builds fail identically)
    • Not caused by this PR's changes
    • runtime pipeline succeeded; 880 cDAC unit tests pass locally

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

@dotnet-policy-service
Copy link
Contributor

Tagging subscribers to this area: @dotnet/runtime-infrastructure
See info in area-owners.md if you want to be subscribed.

…unit and dump tests

Co-authored-by: max-charlamb <44248479+max-charlamb@users.noreply.github.com>
Copilot AI changed the title [WIP] Implement ISOSDacInterface::GetObjectClassName in SOSDacImpl.cs Implement ISOSDacInterface::GetObjectClassName in cDAC SOSDacImpl Feb 24, 2026
Copy link
Member

@max-charlamb max-charlamb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot , address feedback

… case, update tests

Co-authored-by: max-charlamb <44248479+max-charlamb@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Feb 24, 2026

@copilot , address feedback

All feedback addressed in 0239f9f.

Co-authored-by: max-charlamb <44248479+max-charlamb@users.noreply.github.com>
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Implements the cDAC version of ISOSDacInterface::GetObjectClassName in SOSDacImpl, replacing the legacy-only stub, and adds unit + dump-based tests to validate method-table → type/module resolution and string output behavior.

Changes:

  • Add cDAC implementation of GetObjectClassName using IObject, IRuntimeTypeSystem, ILoader, and TypeNameBuilder with a fallback to DacStreams.
  • Add unit tests covering zero address, unloaded-type naming, and “null buffer returns needed size” behavior.
  • Add dump-based integration tests validating that System.Object and System.String resolve to modules with loaded image contents.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs Implements GetObjectClassName in cDAC, including unloaded-module handling, TypeNameBuilder formatting, and DEBUG cross-check vs legacy DAC.
src/native/managed/cdac/tests/ObjectTests.cs Adds unit tests for GetObjectClassName argument handling and unloaded-type naming behavior.
src/native/managed/cdac/tests/DumpTests/RuntimeTypeSystemDumpTests.cs Adds dump tests ensuring Object/String MTs map to modules with loaded image contents via Loader contract.

Copy link
Member

@max-charlamb max-charlamb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changes lgtm

Copy link
Contributor Author

Copilot AI commented Feb 25, 2026

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • https://api.github.com/graphql
    • Triggering command: /usr/bin/gh /usr/bin/gh pr checks 124816 --repo dotnet/runtime (http block)
    • Triggering command: /usr/bin/gh gh pr checks 124816 (http block)

If you need me to access, download, or install something from one of these locations, you can either:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

3 participants